Bound WslCoreInstance init transactions - #41385

test/windows/UnitTests.cpp:7718

• This test doesn’t actually assert that the StartTransaction(200) timeout is being honored: it only checks elapsed >= 100ms, so it could still pass if the implementation incorrectly uses a default (e.g., 100ms) and ignores the 200ms parameter. To make the test validate the intended behavior, assert elapsed time against the configured timeout (with a reasonable tolerance), and consider using named constants for the timeout and tolerances to keep the intent clear and reduce flakiness.

            auto transaction = channel.StartTransaction(200);

test/windows/UnitTests.cpp:7736

• This test doesn’t actually assert that the StartTransaction(200) timeout is being honored: it only checks elapsed >= 100ms, so it could still pass if the implementation incorrectly uses a default (e.g., 100ms) and ignores the 200ms parameter. To make the test validate the intended behavior, assert elapsed time against the configured timeout (with a reasonable tolerance), and consider using named constants for the timeout and tolerances to keep the intent clear and reduce flakiness.

            const auto start = std::chrono::steady_clock::now();
            const auto hr = wil::ResultFromException([&]() { transaction.Receive<RESULT_MESSAGE<int32_t>>(); });
            const auto elapsed =
                std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now() - start).count();

            VERIFY_ARE_EQUAL(hr, HRESULT_FROM_WIN32(ERROR_TIMEOUT));
            VERIFY_IS_GREATER_THAN_OR_EQUAL(elapsed, 100LL);
            VERIFY_IS_LESS_THAN(elapsed, 5000LL);

test/windows/UnitTests.cpp:7718

• The newly added tests introduce several hard-coded timing and value constants (e.g., 200, 1000, 100, 5000, 42). Using constexpr constants (e.g., kTimeoutMs, kMinElapsedMs, kMaxElapsedMs, kExpectedResult) would make the tests easier to understand and adjust, and would reduce the chance of mismatched expectations if timeouts are tweaked later.

            auto transaction = channel.StartTransaction(200);

test/windows/UnitTests.cpp:7736

• The newly added tests introduce several hard-coded timing and value constants (e.g., 200, 1000, 100, 5000, 42). Using constexpr constants (e.g., kTimeoutMs, kMinElapsedMs, kMaxElapsedMs, kExpectedResult) would make the tests easier to understand and adjust, and would reduce the chance of mismatched expectations if timeouts are tweaked later.

            VERIFY_IS_GREATER_THAN_OR_EQUAL(elapsed, 100LL);
            VERIFY_IS_LESS_THAN(elapsed, 5000LL);

test/windows/UnitTests.cpp:7742

• The newly added tests introduce several hard-coded timing and value constants (e.g., 200, 1000, 100, 5000, 42). Using constexpr constants (e.g., kTimeoutMs, kMinElapsedMs, kMaxElapsedMs, kExpectedResult) would make the tests easier to understand and adjust, and would reduce the chance of mismatched expectations if timeouts are tweaked later.

            auto transaction = channel.StartTransaction(1000);

test/windows/UnitTests.cpp:7757

• The newly added tests introduce several hard-coded timing and value constants (e.g., 200, 1000, 100, 5000, 42). Using constexpr constants (e.g., kTimeoutMs, kMinElapsedMs, kMaxElapsedMs, kExpectedResult) would make the tests easier to understand and adjust, and would reduce the chance of mismatched expectations if timeouts are tweaked later.

            response.Result = 42;

src/windows/service/exe/WslCoreInstance.cpp:228

• Passing a raw nullptr as the middle argument makes the call site ambiguous (it’s not obvious what parameter is being intentionally omitted). If the API is expecting an optional/cancellation object, prefer an explicit empty value (e.g., std::nullopt / default-constructed token) or add a small clarifying comment at the call site so future readers don’t have to look up the overload/signature.

        port = sessionLeader->GetChannel().Transaction<LX_INIT_CREATE_PROCESS_UTILITY_VM>(messageSpan, nullptr, m_socketTimeout).Result;

test/windows/UnitTests.cpp:7739

• elapsed is truncated to whole milliseconds via duration_cast(...).count(), which can make this assertion flaky (e.g., a ~199.9ms timeout becomes 199ms and fails the >= check). Consider keeping elapsed as a std::chrono::steady_clock::duration (or at least avoid truncation by using microseconds / rounding up) and compare using std::chrono::milliseconds durations directly.

            const auto start = std::chrono::steady_clock::now();
            const auto hr = wil::ResultFromException([&]() { transaction.Receive<RESULT_MESSAGE<int32_t>>(); });
            const auto elapsed =
                std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now() - start).count();

            VERIFY_ARE_EQUAL(hr, HRESULT_FROM_WIN32(ERROR_TIMEOUT));
            VERIFY_IS_GREATER_THAN_OR_EQUAL(elapsed, static_cast<LONGLONG>(transactionTimeout) - schedulingTolerance);

src/windows/service/exe/WslCoreInstance.cpp:228

• Passing a raw nullptr as the middle argument makes the callsite ambiguous/brittle (it’s not self-documenting what parameter is being omitted). If possible, prefer an overload that takes only (messageSpan, timeout) (or uses a strongly-typed optional/default parameter) so readers don’t have to chase the callee signature; otherwise, add an explicit local variable (e.g., auto cancellation = /*...*/;) or a short comment indicating what nullptr represents.

        port = sessionLeader->GetChannel().Transaction<LX_INIT_CREATE_PROCESS_UTILITY_VM>(messageSpan, nullptr, m_socketTimeout).Result;

src/windows/service/exe/WslCoreInstance.cpp:405

• PR description/issue notes say UnitTests::SocketChannel was extended with Transaction/StartTransaction timeout cases (peer stays connected without replying => HRESULT_FROM_WIN32(ERROR_TIMEOUT), and peer replying before deadline succeeds). I couldn't find any SocketChannel Transaction/StartTransaction timeout tests in the current test sources (only a ReceiveMessage finite-timeout test in test/windows/UnitTests.cpp around lines 7705-7713). Please add the claimed Transaction timeout coverage or update the PR description accordingly.

    auto config = wsl::windows::common::helpers::GenerateConfigurationMessage(
        m_configuration.Name, fixedDrives, m_defaultUid, timezone, {}, m_featureFlags, drvfsMount);

    auto transaction = m_initChannel->GetChannel().StartTransaction(m_socketTimeout);
    transaction.Send<LX_INIT_CONFIGURATION_INFORMATION>(gsl::span(config));